Harden JitteredBackoff: nil-safe, clamp jitter, and seed RNG#1
Harden JitteredBackoff: nil-safe, clamp jitter, and seed RNG#1simonovic86 wants to merge 2 commits intomainfrom
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3a72194de3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
backoff.go
Outdated
| ) | ||
|
|
||
| func init() { | ||
| rand.Seed(time.Now().UnixNano()) |
There was a problem hiding this comment.
Avoid reseeding global math/rand in package init
Calling rand.Seed in init() introduces a process-wide side effect for every consumer that imports this package: it mutates the default RNG used by unrelated code, which can break reproducible tests/simulations and make init-time random behavior depend on import order. This is especially problematic in libraries because callers do not opt into this global mutation, and Go already auto-seeds the default RNG at startup in modern versions.
Useful? React with 👍 / 👎.
Motivation
JitteredBackoffhas a nilStrategyand avoid producing invalid negative durations from excessive jitter.Description
init()withrand.Seed(time.Now().UnixNano())to vary jitter between runs.JitteredBackoff.NextDelayreturn0whenStrategyis nil to avoid panics.JitterRateto a maximum of1.0and ensure the computed jittered delay is never negative by returning0for negative results.TestJitteredBackoff_NilStrategyandTestJitteredBackoff_ClampsNegativeDelayinbackoff_test.goto cover the new edge cases.Testing
go test ./...and all tests passed successfully.go test ./... -run TestSpawnThen_EmptyParallelTasks -vwhich passed.go vet ./...with no issues reported.Codex Task